fix: add on-chain verification and salt nonce increment for Safe deployment #4
Conversation
…oyment - Add on-chain deployment verification before attempting to deploy a Safe - Automatically sync local storage with blockchain state to prevent mismatches - Implement automatic salt nonce increment when creating Safes with duplicate configs - Prevent address collisions by checking up to 100 salt nonce variations - Store salt nonce in predictedConfig for deterministic address generation This resolves issues where: 1. Users couldn't deploy Safes that local storage incorrectly marked as deployed 2. Users couldn't create multiple Safes with identical owner/threshold configurations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
This PR enhances Safe account deployment reliability by adding on-chain verification and automatic salt nonce increment functionality. It addresses two critical issues: local storage inconsistencies where Safes were marked as deployed without actual on-chain deployment, and address collisions when creating multiple Safes with identical configurations.
Key changes:
- On-chain verification before deployment attempts with automatic local storage synchronization
- Salt nonce increment loop (0→100 max attempts) to find available addresses when configurations collide
- Storage of salt nonce in predictedConfig for deterministic deployment
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/commands/account/deploy.ts | Adds on-chain deployment verification via SafeService.getSafeInfo() and syncs local storage state with blockchain reality before deployment attempts |
| src/commands/account/create.ts | Implements salt nonce increment loop to prevent address collisions and stores salt nonce in predictedConfig |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!safe.predictedConfig) { | ||
| logError('Safe does not have deployment configuration') | ||
| return | ||
| } | ||
|
|
||
| // Verify on-chain deployment status | ||
| const chain = configStore.getChain(safe.chainId)! | ||
| const safeService = new SafeService(chain) | ||
|
|
||
| try { | ||
| const safeInfo = await safeService.getSafeInfo(address) |
There was a problem hiding this comment.
The order of checks has been changed so that predictedConfig is now verified before checking on-chain deployment status. This creates inconsistent logic: the on-chain verification runs even when predictedConfig is missing, but deployment requires predictedConfig. Consider moving the predictedConfig check after the on-chain verification, or restructure to check both predictedConfig and on-chain status before proceeding with any logic that depends on either.
| if (attempts >= maxAttempts || !predictedAddress || !safeAccountConfig) { | ||
| throw new Error(`Could not find available Safe address after ${maxAttempts} attempts`) | ||
| } |
There was a problem hiding this comment.
The error message doesn't distinguish between 'max attempts exceeded' and 'missing predictedAddress/safeAccountConfig'. The latter would indicate a logic error rather than exhausted attempts. Consider separate error messages: one for exceeding max attempts, and another for unexpected missing data.
| if (attempts >= maxAttempts || !predictedAddress || !safeAccountConfig) { | |
| throw new Error(`Could not find available Safe address after ${maxAttempts} attempts`) | |
| } | |
| if (attempts >= maxAttempts) { | |
| throw new Error(`Could not find available Safe address after ${maxAttempts} attempts`) | |
| } | |
| if (!predictedAddress || !safeAccountConfig) { | |
| throw new Error('Unexpected error: missing predictedAddress or safeAccountConfig after finding available Safe address') | |
| } |
Summary
Changes
Safe Deployment (
src/commands/account/deploy.ts)SafeService.getSafeInfo()before deploymentdeployedflag with blockchain stateSafe Creation (
src/commands/account/create.ts)predictedConfigfor deterministic deploymentFixes
Test Plan
🤖 Generated with Claude Code